-
Notifications
You must be signed in to change notification settings - Fork 798
Feature: Automatic Profile & Settings backup #3302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds configurable backup retention for both application settings and profile files. Users can now enable/disable daily automatic backups and configure the maximum number of backups to retain (1-365, default 10). The PR implements a significant refactoring of the ProfileManager to support per-profile-file backup tracking through a new ProfileFileData wrapper class.
Changes:
- Added UI controls (toggle switch and numeric input) to configure backup settings for both Settings and Profiles in the application's settings views
- Refactored ProfileManager from using a static
Groupslist to aProfileFileDatawrapper that includes metadata (version, last backup date) alongside the groups - Updated all ViewModel references from
ProfileManager.GroupstoProfileManager.LoadedProfileFileData.Groups - Enhanced backup logic in both SettingsManager and ProfileManager to check if backups are enabled before creating them
- Updated documentation to describe the new configuration options
Reviewed changes
Copilot reviewed 119 out of 121 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| Website/docs/settings/settings.md | Documents new backup configuration options for settings |
| Website/docs/settings/profiles.md | Documents new backup configuration options for profiles |
| Website/docs/changelog/next-release.md | Updates changelog to reference this PR |
| Source/NETworkManager/Views/SettingsSettingsView.xaml | Adds UI controls for backup configuration |
| Source/NETworkManager/Views/SettingsProfilesView.xaml | Adds UI controls for backup configuration |
| Source/NETworkManager/ViewModels/SettingsSettingsViewModel.cs | Implements backup settings properties with loading guard |
| Source/NETworkManager/ViewModels/SettingsProfilesViewModel.cs | Implements backup settings properties with loading guard |
| Source/NETworkManager/ViewModels/*HostViewModel.cs (multiple) | Updates references from ProfileManager.Groups to LoadedProfileFileData.Groups |
| Source/NETworkManager/ViewModels/ProfilesViewModel.cs | Updates profile group access to use new data structure |
| Source/NETworkManager.Settings/SettingsInfo.cs | Adds properties for backup enable/disable and max count |
| Source/NETworkManager.Settings/GlobalStaticConfiguration.cs | Moves backup defaults to profile/settings-specific constants |
| Source/NETworkManager.Settings/SettingsManager.cs | Adds backup enable check and configurable retention |
| Source/NETworkManager.Profiles/ProfileManager.cs | Major refactoring to use ProfileFileData wrapper with backup tracking |
| Source/NETworkManager.Profiles/ProfileFileData.cs | New class wrapping groups with version and backup metadata |
| Source/NETworkManager.Profiles/ProfileInfoSerializable.cs | Adds documentation comments |
| Source/NETworkManager.Profiles/GroupInfoSerializable.cs | Minor whitespace cleanup |
| Source/NETworkManager.Profiles/GroupInfo.cs | Whitespace cleanup |
| Source/NETworkManager.Utilities/TimestampHelper.cs | Modernizes string slicing syntax |
| Source/NETworkManager/App.xaml.cs | Updates save logic to handle new ProfileFileData structure |
| Source/NETworkManager.Localization/Resources/Strings.resx | Adds localization strings for new UI elements |
| Source/NETworkManager.Localization/Resources/Strings.Designer.cs | Auto-generated localization code |
| Multiple ViewModels (60+ files) | Removes BOM character from file start |
Files not reviewed (1)
- Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var backupFiles = Directory.GetFiles(backupFolderPath) | ||
| .Where(f => (f.EndsWith(settingsFileName) || f.EndsWith(GetLegacySettingsFileName())) && TimestampHelper.IsTimestampedFilename(Path.GetFileName(f))) | ||
| .Where(f => | ||
| { | ||
| var fileName = Path.GetFileName(f); | ||
|
|
||
| // Check if it's a timestamped backup and contains the settings name | ||
| return TimestampHelper.IsTimestampedFilename(fileName) && | ||
| fileName.Contains($"_{settingsNameWithoutExtension}."); | ||
| }) | ||
| .OrderByDescending(f => TimestampHelper.ExtractTimestampFromFilename(Path.GetFileName(f))) | ||
| .ToList(); |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CleanupBackups method in SettingsManager does not check if the backup directory exists before attempting to get files from it. This could result in a DirectoryNotFoundException if the directory doesn't exist. The ProfileManager implementation at line 1280-1284 includes this check and logs an error if the directory is missing. Consider adding the same check here for consistency and robustness.
| <TextBlock Text="{x:Static localization:Strings.MaximumNumberOfBackups}" | ||
| Style="{StaticResource CenterTextBlock}" | ||
| Margin="0,0,0,10" /> | ||
| <StackPanel Orientation="Horizontal" Margin="0,0,0,20"> |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has trailing whitespace after "Horizontal" and before the closing angle bracket. Consider removing it for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
| <Button Content="{x:Static localization:Strings.Reset}" Command="{Binding ResetSettingsCommand}" | ||
| IsEnabled="{Binding SettingsExists}" Style="{StaticResource DefaultButton}" HorizontalAlignment="Left" /> | ||
| Style="{StaticResource DefaultButton}" HorizontalAlignment="Left" | ||
| /> |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This closing tag has trailing whitespace before the closing angle bracket and is split across two lines unnecessarily. Consider placing it on a single line for consistency with the rest of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@BornToBeRoot I've opened a new pull request, #3307, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@BornToBeRoot I've opened a new pull request, #3308, to work on those changes. Once the pull request is ready, I'll request review from you. |
…3308) * Initial plan * Fix: Consolidate Button closing tag to single line Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>
* Initial plan * Remove trailing whitespace from SettingsSettingsView.xaml line 40 Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>
…om/BornToBeRoot/NETworkManager into feature/configure_backup_retention
Changes proposed in this pull request
Related issue(s)
To-Do
Contributing
By submitting this pull request, I confirm the following: